-
Notifications
You must be signed in to change notification settings - Fork 442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rust_analyzer: generate more reqired sources #3031
base: main
Are you sure you want to change the base?
Conversation
The existing support for this has limitations: - only works for sources that are specially tagged (rust_generated_srcs). I've removed this mechanism as it's no longer needed. - only provides sources of the directly selected library, but rust-analyzer needs to parse its dependencies too. - is missing the target.json file, which rust-analyzer requires to parse the library (it invokes rustc --print cfg --target=...) The dependency depth to generate sources for is a tradeoff. More layers of deps will give more accurate analysis but be slower to generate. My guess is that in the end we'll want full transitive sources, but direct deps should be a non-controversial starting point. (Without these sources, rust-analyzer can't understand the direct API usage at all). We can change this later and probably need to reconcile with bazelbuild#3028.
The changes to rust_analyzer look good. I'm not sure if the It would be nice to add some tests to make sure that the expected sources are made available to the expected actions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(sorry I left a comment in the wrong review state) The changes to rust_analyzer look good. I'm not sure if the rust_generated_srcs output group is used for anything else though?
It would be nice to add some tests to make sure that the expected sources are made available to the expected actions.
It's not used for anything else.
I could extend generate_srcrs_test to add some. The problem is that the test driver runs I can try to remove that behavior from the test driver, but I don't understand why it does that to start with. Or I can pile hacks on top (more test tags). What would you prefer? |
Seems like getting the bash-style test to reliably test using generated vs. non-generated files might be tricky. |
The existing support for this has limitations:
I've removed this mechanism as it's no longer needed.
rust-analyzer needs to parse its dependencies too.
parse the library (it invokes rustc --print cfg --target=...)
The dependency depth to generate sources for is a tradeoff. More layers
of deps will give more accurate analysis but be slower to generate.
My guess is that in the end we'll want full transitive sources, but
direct deps should be a non-controversial starting point. (Without these
sources, rust-analyzer can't understand the direct API usage at all).
We can change this later and probably need to reconcile with #3028.